use property for gcc_prefix/gcc_root in LLVM easyblock - #3793
Conversation
d57c49b to
dc7c566
Compare
Flamefire
changed the title
Flamefire
changed the title
|
Test report by @Flamefire Overview of tested easyconfigs (in order)
Build succeeded for 2 out of 3 (3 easyconfigs in total) |
|
Test report by @Flamefire Overview of tested easyconfigs (in order)
Build succeeded for 2 out of 3 (3 easyconfigs in total) |
|
@Flamefire Merge conflict to fix now that #3817 is merged? |
boegel
changed the title
|
Rebased |
|
Test report by @Flamefire Overview of tested easyconfigs (in order)
Build succeeded for 2 out of 3 (3 easyconfigs in total) |
|
Test report by @Flamefire Overview of tested easyconfigs (in order)
Build succeeded for 3 out of 3 (3 easyconfigs in total) |
|
Test report by @Flamefire Overview of tested easyconfigs (in order)
Build succeeded for 3 out of 3 (3 easyconfigs in total) |
|
|
||
| def _set_gcc_prefix_probs(self): | ||
| """Set properties of currently loaded GCC installation""" | ||
| self._gcc_root, self._gcc_prefix = self._get_gcc_prefix() |
There was a problem hiding this comment.
I would put this inside an if as it was done originally.
I do not foresee any situation where these variables would change during the build process
There was a problem hiding this comment.
This is a method only used on access of the properties to set the "backing storage". So the if is outside this function
I agree they won't change, so there is no harm if this method is (wrongly) called again
There was a problem hiding this comment.
There is no if either inside gcc_prefix nor gcc_root so the method self._get_gcc_prefix() will be called every time one of these 2 properties are requested
There was a problem hiding this comment.
You are right, thanks! Seems I couldn't decide where to put the if and forgot it in the end.
Added
|
@boegelbot please test @ jsc-zen3 |
|
@Crivella: Request for testing this PR well received on jsczen3l1.int.jsc-zen3.fz-juelich.de PR test command '
Test results coming soon (I hope)... Details- notification for comment with ID 3051630099 processed Message to humans: this is just bookkeeping information for me, |
|
Test report by @boegelbot Overview of tested easyconfigs (in order)
Build succeeded for 4 out of 4 (4 easyconfigs in total) |
|
Going in, thanks @Flamefire! |
4 participants
As discussed this uses properties that auto-initialize on (first) use to avoid them being used before setting in
_set_gcc_prefixwhich acted as a configure-method AND setting those propertiesBUGFIX (Separated out to #3817 but would cause a conflict)
For
bootstrap = Falsethe function_create_compiler_config_filewas called without checking the version which results in adding a CLI argument for clang compilers that was introduced in LLVM 19 which causes failures.